Make BFLB not use the SDK code #91977

VynDragon · 2025-06-21T19:54:04Z

What's in the title.

Large update that cannot be cut without breaking functionality.

Some drivers are getting entirely revamped, such as uart and pinctrl, do not consider those as updates of the older drivers, but rather new drivers (which is true to some large extent).

soc/bflb/bl60x/soc.c

Copilot

Pull Request Overview

This PR removes reliance on the Bouffalo Lab SDK code and replaces it with custom, in-tree drivers and build logic tailored for the BL60x series.

Deletes legacy vector, IRQ, and common interrupt files in favor of new “e24” implementations
Updates SOC headers and initialization routines to use sys_read32/sys_write32 instead of HAL wrappers
Overhauls the UART and pinctrl drivers, Kconfig, CMakeLists, and DTS bindings for custom flash partition support

Reviewed Changes

Copilot reviewed 42 out of 42 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
soc/bflb/common/vector.S	Removed legacy reset vector assembly
soc/bflb/common/soc_irq.S	Removed old IRQ handler assembly
soc/bflb/common/soc_common_irq.c	Deleted common CLIC-based interrupt code
soc/bflb/common/soc_common.h	Removed shared interrupt number definitions
soc/bflb/common/e24/soc_irq_privileged.c	Added new privileged IRQ driver
soc/bflb/common/e24/intc_clic.S	Added CLIC interrupt assembly hooks
soc/bflb/common/e24/clic.h	Defined new CLIC CSR registers
soc/bflb/common/CMakeLists.txt	Switched source inclusion to `e24` under BL60x
soc/bflb/bl60x/soc.h	Dropped SDK header include; now minimal SOC defs
soc/bflb/bl60x/soc.c	Rewrote early init to use `sys_read32`/`sys_write32`
soc/bflb/bl60x/CMakeLists.txt	Updated linker script and code relocation calls
modules/hal_bouffalolab/{include,src,CMakeLists}	Removed SDK HAL headers/sources; simplified include paths
drivers/serial/uart_bflb.c	Major rewrite to use register‐access APIs
drivers/pinctrl/pinctrl_bflb[.c,_bl60x_70x.c]	Switched pinctrl logic to `sys_read32`/`sys_write32`
drivers/clock_control/clock_control_bl60x.c	Increased NOP settle loops; moved settle call
dts/riscv/bflb/bl60x.dtsi	Updated interrupt-parent to CLIC and flash dividers
boards/*/.dts	Replaced legacy SPI controller with flash partitions
boards/*/support/.cfg	Adjusted OpenOCD adapter speeds and reset regs

Comments suppressed due to low confidence (1)

modules/hal_bouffalolab/CMakeLists.txt:6

This block now only adds include directories and no longer compiles any HAL sources for Bouffalo Lab, likely disabling all HAL functionality. Restore zephyr_library_sources for enabled modules or adjust the condition.

if(CONFIG_SOC_FAMILY_BFLB)

Copilot · 2025-07-06T16:16:42Z

soc/bflb/common/CMakeLists.txt

@@ -4,8 +4,9 @@

 zephyr_include_directories(.)

+if(CONFIG_SOC_SERIES_BL60X)


Removing vector.S, soc_irq.S, and soc_common_irq.c unconditionally and only adding e24 sources under CONFIG_SOC_SERIES_BL60X will break startup and IRQ handling for non-BL60X SoCs. Consider wrapping legacy sources in an else block or guarding removals similarly.

Copilot · 2025-07-06T16:16:43Z

drivers/serial/uart_bflb.c

-	PINCTRL_DT_INST_DEFINE(n);						\
+
+#define BFLB_UART_INIT(instance)						\
+	PINCTRL_DT_INST_DEFINE(instance);					\
 	PM_DEVICE_DT_INST_DEFINE(n, uart_bflb_pm_control);			\


The BFLB_UART_INIT macro uses n here instead of the macro parameter instance, causing incorrect device definitions. Replace n with instance.

Suggested change

PM_DEVICE_DT_INST_DEFINE(n, uart_bflb_pm_control); \

PM_DEVICE_DT_INST_DEFINE(instance, uart_bflb_pm_control); \

Copilot · 2025-07-06T16:16:43Z

drivers/pinctrl/pinctrl_bflb.c

-					  + BFLB_PINMUX_GET_SIGNAL(pins[i])
-					 );
+
+		if ((BFLB_PINMUX_GET_FUN(pins[i]) & 0xFF) == BFLB_PINMUX_FUN_INST_uart0) {


[nitpick] Masking with 0xFF to extract function bits is a magic operation. Consider defining a named constant or helper to clarify the intent and ensure future compatibility.

Suggested change

if ((BFLB_PINMUX_GET_FUN(pins[i]) & 0xFF) == BFLB_PINMUX_FUN_INST_uart0) {

if ((BFLB_PINMUX_GET_FUN(pins[i]) & BFLB_PINMUX_FUN_MASK) == BFLB_PINMUX_FUN_INST_uart0) {

nandojve · 2025-07-06T16:17:45Z

Because merely having the sdk built (and so having the pinctrl driver from it) is likely to break everything else, then everything is interdependent (uart needs clock control working, interrupts need soc update, etc), and mostly i don't want to spend 5-10 hours working out a transition that reduces the review time by ~10 minutes (or rather the idea of the review time, because imo having a coherent changeset and not having to check 3 different sets make it faster), at some point demands got to be symmetrical, and I highly doubt any reviewer beside you will do anything but quickly check over the code.

Also looking at other sets, it's not even big for basically reintroducing a SoC

Unless you have crystal ball or a oracle you should not try to guess people opinions.

nandojve

Your sequence do not bisect. You must fixed.

nandojve · 2025-07-06T16:15:18Z

drivers/pinctrl/pinctrl_bflb_bl60x_70x.c

+#if defined(CONFIG_SOC_SERIES_BL60X)
+#include <zephyr/dt-bindings/pinctrl/bl60x-pinctrl.h>
+#endif


Add an else to generate an error

VynDragon · 2025-07-06T17:11:00Z

Your sequence do bisect. You must fixed.

What does that mean here?

nandojve · 2025-07-06T18:27:54Z

Your sequence do bisect. You must fixed.

What does that mean here?

Each commit must build individually otherwise you broke the git bisect operation.

VynDragon · 2025-07-06T18:38:20Z

Your sequence do bisect. You must fixed.

What does that mean here?

Each commit must build individually otherwise you broke the git bisect operation.

Yea it's easier to get once the english is fixed.

Why do you think those commits are inseparable? They cannot be made to compile in separate PRs, they cannot be made to compile individually, would you rather I squash them?

kartben · 2025-07-06T19:41:20Z

drivers/pinctrl/pinctrl_bflb_bl60x_70x.c

+
+	/* Disable output anyway */
+	regval = sys_read32(GLB_BASE + GLB_GPIO_CFGCTL34_OFFSET + ((real_pin >> 5) << 2));
+	regval &= ~(1 << (pin & 0x1f));


uh, yea? Would you prefer something like 'pin_number' ?

Nah I mean this line should be s/pin/real_pin/?

pin is already used as the input to the function, this is the pin number extracted from that

Ah no wait sorry, let me check. 1f should be function

You were correct, it's the output enable setting by pin number and it's a overflow prevention mask.

kartben · 2025-07-07T07:47:21Z

drivers/serial/uart_bflb.c


 	if (data->user_cb) {
 		data->user_cb(dev, data->user_data);
 	}
+	/* clear interrupts that require ack*/
+	tmp = sys_read32(cfg->base_reg + UART_INT_CLEAR_OFFSET);
+	tmp = tmp | UART_CR_URX_RTO_CLR;


I think there's more that needs clearing, e.g. you've enabled PCE so UART_CR_URX_PCE_CLR ; probably UART_CR_URX_END_CLR and UART_CR_UTX_END_CLR as well?

Okay no, deleted message was right but i looked at the wrong place UART_CR_URX_END_CLR and UART_CR_UTX_END_CLR 's interrupts should be masked and UART_CR_URX_PCE_CLR is handled in uart_bflb_err_check

(masked = interrupt not thrown to CPU, but i think it was required for working?)

kartben · 2025-07-07T08:02:49Z

soc/bflb/common/e24/soc_irq_privileged.c

+void riscv_clic_irq_priority_set(unsigned int irq, unsigned int prio, uint32_t flags)
+{
+	ARG_UNUSED(irq);
+	ARG_UNUSED(prio);
+	ARG_UNUSED(flags);
+}


would be good to document why it's not being implemented? Or... should it be implemented?

It should probably yes, will look into it.

Shell sample isnt working on this PR while it should anyway so i have some checks to do...

It works with debugger.... uuuurgh

fixing this has had the knock-off effect of somehow fixing the shell sample without the little wait I just added... Sometimes I really dont understand those chips...

kartben · 2025-07-07T08:05:09Z

drivers/serial/uart_bflb.c

+	tmp = sys_read32(cfg->base_reg + UART_STATUS_OFFSET);
+	rc = tmp & UART_STS_UTX_BUS_BUSY;
+	tmp = sys_read32(cfg->base_reg + UART_FIFO_CONFIG_1_OFFSET);
+	rc += tmp & UART_TX_FIFO_CNT_MASK;
+	return (rc > 0 ? 1 : 0);


i think you have the logic backwards?
irq_tx_complete returns 1 If nothing remains to be transmitted ; 0 If transmission is not completed.

tx is probably not complete if UART_STS_UTX_BUS_BUSY is set or if FIFO still contains data?

Sounds like it yes

Done

Note tx fifo count is inverted, it counts free bytes.

kartben · 2025-07-07T08:21:02Z

drivers/serial/uart_bflb.c

+int uart_bflb_irq_tx_ready(const struct device *dev)
+{
+	const struct bflb_config *const cfg = dev->config;
+	uint32_t tmp = 0;
+
+	tmp = sys_read32(cfg->base_reg + UART_FIFO_CONFIG_1_OFFSET);
+	return (tmp & UART_TX_FIFO_CNT_MASK) > 0;
+}
+
+int uart_bflb_irq_rx_ready(const struct device *dev)
+{
+	const struct bflb_config *const cfg = dev->config;
+	uint32_t tmp = 0;
+
+	tmp = sys_read32(cfg->base_reg + UART_FIFO_CONFIG_1_OFFSET);
+	return (tmp & UART_RX_FIFO_CNT_MASK) > 0;
+}
+


you've setup thresholds so raw count shouldn't be used? I think this should instead check if UART_URX_FIFO_INT / UART_UTX_FIFO_INT are set/pending.

These interrupts are masked, functionally it's the same (And there might have been a reason, interrupts gave me, and continue to give me, a lot of trouble on this platform, I wouldnt remember but it's possible there was issues doing it with the interrupt).

kartben · 2025-07-07T08:25:03Z

boards/aithinker/ai_wb2_12f/support/bl60x.cfg

-    reg mstatus 0x7800
-    reg mie 0x0
-#    reg pc 0x23000000
+    reg mstatus 0x80000000


bit 31 of mstatus is read-only?

Not sure where that's from, I see the same, maybe 0x0 is more appropriate.

Reorganize and update soc folder files for SDK-independance Signed-off-by: Camille BAUD <mail@massdriver.space>

Reorganize and update hal_bouffalolab files for SDK-independance Signed-off-by: Camille BAUD <mail@massdriver.space>

Reorganize and update soc files for SDK-independance Signed-off-by: Camille BAUD <mail@massdriver.space>

Update driver file for SDK-independance Signed-off-by: Camille BAUD <mail@massdriver.space>

mostly makes things a little bit safer Signed-off-by: Camille BAUD <mail@massdriver.space>

Update to new bl60x support and fixup openocd config Signed-off-by: Camille BAUD <mail@massdriver.space>

Update to new bl60x support Signed-off-by: Camille BAUD <mail@massdriver.space>

update to new bl60x support Signed-off-by: Camille BAUD <mail@massdriver.space>

sonarqubecloud · 2025-07-07T11:57:51Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

VynDragon requested a review from nandojve June 21, 2025 19:54

VynDragon changed the title ~~Make BFLB not used the SDK code~~ Make BFLB not use the SDK code Jun 21, 2025

VynDragon force-pushed the bl60x_nosdk branch 5 times, most recently from 6e0c76d to f2a098f Compare June 21, 2025 20:59

kartben reviewed Jun 21, 2025

View reviewed changes

soc/bflb/bl60x/soc.c Outdated Show resolved Hide resolved

VynDragon force-pushed the bl60x_nosdk branch 4 times, most recently from 175cf3f to 324da9a Compare June 22, 2025 12:30

VynDragon marked this pull request as ready for review June 22, 2025 13:13

github-actions bot added area: Pinctrl area: Interrupt Controller platform: bouffalolab area: UART Universal Asynchronous Receiver-Transmitter area: RISCV RISCV Architecture (32-bit & 64-bit) area: Flash platform: Bouffalo Lab area: Clock Control labels Jun 22, 2025

github-actions bot requested review from carlocaione, dcpleung, de-nordic, fkokosinski, gmarull, kgugala, mgielda, nashif and nordic-krch June 22, 2025 13:14

VynDragon force-pushed the bl60x_nosdk branch from 324da9a to 149261d Compare July 6, 2025 16:04

kartben requested a review from Copilot July 6, 2025 16:14

Copilot AI reviewed Jul 6, 2025

View reviewed changes

nandojve requested changes Jul 6, 2025

View reviewed changes

VynDragon force-pushed the bl60x_nosdk branch 3 times, most recently from d70de46 to 4a5f872 Compare July 6, 2025 17:36

VynDragon force-pushed the bl60x_nosdk branch from 4a5f872 to 0dd5dcf Compare July 6, 2025 18:44

kartben reviewed Jul 6, 2025

View reviewed changes

VynDragon force-pushed the bl60x_nosdk branch from 0dd5dcf to 9334d9a Compare July 6, 2025 20:22

kartben reviewed Jul 7, 2025

View reviewed changes

VynDragon force-pushed the bl60x_nosdk branch from 9334d9a to e57cb5e Compare July 7, 2025 10:26

VynDragon added 4 commits July 7, 2025 12:40

soc: Make BL60x independant from SDK

d325362

Reorganize and update soc folder files for SDK-independance Signed-off-by: Camille BAUD <mail@massdriver.space>

modules: hal_bouffalolab: Make sdk-independant

3bad2c8

Reorganize and update hal_bouffalolab files for SDK-independance Signed-off-by: Camille BAUD <mail@massdriver.space>

dts: Update bl60x and relevant files for SDK-independance

36ed3e2

Reorganize and update soc files for SDK-independance Signed-off-by: Camille BAUD <mail@massdriver.space>

drivers: pinctrl: Update bflb pinctrl driver

bf73fa5

Update driver file for SDK-independance Signed-off-by: Camille BAUD <mail@massdriver.space>

VynDragon force-pushed the bl60x_nosdk branch from e57cb5e to 418a18c Compare July 7, 2025 10:40

VynDragon added 5 commits July 7, 2025 13:32

drivers: serial: make bflb uart driver independent from SDK

4d28783

Update driver file for SDK-independance Signed-off-by: Camille BAUD <mail@massdriver.space>

drivers: misc bflb updates

946d235

mostly makes things a little bit safer Signed-off-by: Camille BAUD <mail@massdriver.space>

boards: ai_wb2_12f: update to new bl60x support

523119b

Update to new bl60x support and fixup openocd config Signed-off-by: Camille BAUD <mail@massdriver.space>

boards: bl604e_iot_dvk: update to new bl60x support

06baa94

Update to new bl60x support Signed-off-by: Camille BAUD <mail@massdriver.space>

boards: dt_bl10_dvk: update to new bl60x support

382a12e

update to new bl60x support Signed-off-by: Camille BAUD <mail@massdriver.space>

VynDragon force-pushed the bl60x_nosdk branch from 418a18c to 382a12e Compare July 7, 2025 11:32

VynDragon requested review from kartben and nandojve July 10, 2025 09:06

		@@ -4,8 +4,9 @@

		zephyr_include_directories(.)

		if(CONFIG_SOC_SERIES_BL60X)

	PM_DEVICE_DT_INST_DEFINE(n, uart_bflb_pm_control); \
	PM_DEVICE_DT_INST_DEFINE(instance, uart_bflb_pm_control); \

	if ((BFLB_PINMUX_GET_FUN(pins[i]) & 0xFF) == BFLB_PINMUX_FUN_INST_uart0) {
	if ((BFLB_PINMUX_GET_FUN(pins[i]) & BFLB_PINMUX_FUN_MASK) == BFLB_PINMUX_FUN_INST_uart0) {

Make BFLB not use the SDK code #91977

Are you sure you want to change the base?

Make BFLB not use the SDK code #91977

Uh oh!

Conversation

VynDragon commented Jun 21, 2025

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jul 6, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 6, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 6, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nandojve commented Jul 6, 2025

Uh oh!

nandojve left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

VynDragon commented Jul 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nandojve commented Jul 6, 2025

Uh oh!

VynDragon commented Jul 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kartben Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nandojve left a comment •

edited

Loading

VynDragon commented Jul 6, 2025 •

edited

Loading

VynDragon commented Jul 6, 2025 •

edited

Loading

kartben Jul 7, 2025 •

edited

Loading

VynDragon Jul 7, 2025 •

edited

Loading